-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Mention named givens in double def explainer #23833
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
As mentioned at #23121, there are other cases of clashing invented names that are not double defs because the for comprehension desugars them to locals and pattern vars. |
Print info at typer in example code. Could be automatic.
e6c9c95
to
6fe3abc
Compare
The purpose of this extra help is to clarify that "anonymous" givens have names, albeit invented ones, and that it turns out to be necessary to name at least one of them explicitly. The example is to prompt the user as to what such a name looks like. Maybe they have never written a named given. Hopefully the example gives them enough information to prompt copilot in turn. That is, it is not a goal of the addendum to be complete or pastable. It ought to be approximately correct. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| | ||
| @targetName("given_Special_Option_2") | ||
| final lazy given val given_Special_Option: Special[Option[Int]] | ||
| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is 3rd option on how to name them, specifically in for-comprehension (it could shadow previous one):
for {
// ...
givenName @ given Type <- ???
// ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mr-git that is a different error ID, though a similar explainer about "how to name givens" may be warranted (since it is not obvious); presumably it is obvious how to rename other things?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will you create a ticket and PR for that? I struggled with this case the most.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mr-git I added some explain text on the other PR. There is no pattern syntax for given here because there is no equivalent to val Extract(x, y) = expr
.
| In your code the two declarations | ||
| | ||
| final lazy given val given_Special_Option: Special[Option[Long]] | ||
| final lazy given val given_Special_Option: Special[Option[Int]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't it be something like:
final lazy given Special[Option[Long]] = ???
final lazy given Special[Option[Int]] = ???
which leads to 2 identical signatures: given_Special_Option
?
When Developer names the val
s, it is very obvious which names the Developer gives them - problems usually start with non-obvious auto-naming.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The declarations are not printed with "fidelity". That could be fixed as a follow-up. (I assume but did not check that the syntax changed faster than the printer.)
| | ||
| To fix this error, you must disambiguate the two definitions by doing one of the following: | ||
| | ||
| 1. Rename one of the definitions. Provide an explicit, unique name to given definitions, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe Rename one or all the definitions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All these errors fail at the first pair that is noticed; and "already defined" in particular works with just minimal info; it does not try to enumerate all problematic definitions and their types. The spec will sometimes say "pairwise distinct", and the user only needs to fix one pair at a time.
| | ||
| given myGiven: Special[Option[Int]] | ||
| | ||
| 2. Keep the same names in source but give one definition a distinct |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and ..., but give one or to all of them a distinct
- like we should not suggest to rename only one of them - sometimes renaming them all makes more sense and improves consistency
| | ||
| @targetName("given_Special_Option_2") | ||
| final lazy given val given_Special_Option: Special[Option[Int]] | ||
| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will you create a ticket and PR for that? I struggled with this case the most.
Print info at typer in example code. Could be automatic.
Note the sig test must not be at typer.
Fixes #23832